Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

NFT sketching #366

Closed
wants to merge 4 commits into from
Closed

NFT sketching #366

wants to merge 4 commits into from

Conversation

liamzebedee
Copy link
Contributor

@liamzebedee liamzebedee commented Nov 22, 2019

  • isQualified function
  • locking mechanism a la Maker

implementation planing: #344

function draw(uint amt) {
require(amt == 1 tbtc, "partial draw not permitted");
mintTbtc(msg.sender, amt);
drawn += amt;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this make sense to track in the deposit at all? One of the advantages of the vending machine to me is that the deposit needn't know about the token; all it's concerned with is what state the backing UTXO and signers are in.

In my mind, it makes more sense for the deposit to track its own transaction's proof state (e.g., I have/have not minted an NFT with 1 vs 2 vs 8 vs 1000 confirmations, etc). Proving more work can be done on the deposit, and the deposit can report how much work has been proven on its backing UTXO. I'm still getting my sea legs with Bitcoin terminology, so please don't hurt me if that sounds like a 5-year-old's mixing up of terms---hoping the core concept gets across.

Notably, both this sketch and #364 imply the vending machine is doing qualification. Instead, I think the vending machine can, given an NFT, ask its backing deposit “hey, can you meet these proof requirements?” Then the vending machine may want to provide some convenience code that lets you say “here is my deposit NFT + some additional proof”, but the proof is ultimately passed through to the deposit backing the NFT for verification + recording.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this make sense to track in the deposit at all?

Don't think tracking this in Deposit is the way to go. Took a while thinking about implications for having a Drawn variable or state. I'm not fully clear on whether this is intended to allow fractional drawing on larger lot sizes but assuming require(amt == 1) == require(amt == lot Size)
A benefit of the vending machine is that it also restricts minting authority to just one contract. A simple query on the NFT and Deposit state can give the vending machine all the info it needs to mint or refuse minting.

In my mind, it makes more sense for the deposit to track its own transaction's proof state

Sitting with this for a while, I think I agree. This makes sense thematically and in terms of implementation. The proof state is technically knowledge of backing UTXO's proven work state, and can justifiably be stored in deposit state. Deposit's 1-proof functionality will also be very similar to Vending Machine's 6+x-proof therefore We can cut a lot of duplicated complexity from the Vending machine.
Note: This needs to be a public / external function callable only via the vending machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design-wise, I tend to agree with what you've said. Been doing some iteration today and I see that it's generally cleaner to centralise our token minting/burning in the vending machine.

it makes more sense for the deposit to track its own transaction's proof state

Mmmmm. To follow on from that point, the vending machine only needs to know the BTC volume of unqualified TBTC flowing through. The actual qualification can be computed in the Deposit itself. This makes it a little easier too, in case we support different types of proofs ie. Lightning payments (#262).

drawn -= amt;
}

function setLocked(bool _locked) {
Copy link
Contributor

@Shadowfiend Shadowfiend Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notably, in the model I mention above, ownership transfers to the vending machine when an NFT is put in the vending machine, and locked vs unlocked might be a deposit state (one which the deposit moves out of during liquidation). If we went the deposit state path, we'd probably have four total states: ACTIVE_UNLOCKED, ACTIVE_LOCKED, COURTESY_CALL_UNLOCKED, and COURTESY_CALL_LOCKED (or something like that), which ain't great but feels clearer than tracking additional state outside of the state machine proper.

There's an open question on the state side: can a deposit move between locked or unlocked during courtesy call? My take would be “sure, why not”, but interested in thoughts there as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one more open thing here that I realized in chatting with @NicholasDotSol on his draft: we need the vending machine to be able to start locked deposit redemption, probably.

There are really two ways to architect this: you send TBTC to the deposit and it hands it to the vending machine for burning, or you send TBTC to the vending machine and it has the deposit notify it of when to burn. The latter is simplest in that it is the same flow for locked and unlocked deposits, except that the vending machine and deposit both verify, for locked deposits, that the redeemer is the owner. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

locked vs unlocked might be a deposit state
👍

can a deposit be locked or unlocked during courtesy call?

This is tough. quick term Deposit Owner Token = DOT
some points:
Unlocked & redeemed Deposits:

  • This is not an issue for unlocked deposits

Locked Deposits:

  • redeemed & locked Deposits: Owner controls TBTC and a redeemed DOT. If the state of the DOT (redeemed / unredeemed) is just a reflection of the contract state(as you mentioned @Shadowfiend), we can change the state of the Deposit (and therefore the DOT) during an undercollateralized state without breaching permission. The result is that the user keeps their TBTC, but loses exclusive redemption rights making their DOT worthless (since it just points to a liquidated deposit).
    TLDR - when undercolateralized, liquidation actions are open and operate as they do in the current spec. Deposit holds all state, not the NFT.
  • unredeemed Deposits: I'm not entirely sure what the best approach is here. In this situation, owner does not control any TBTC. Liquidating the linked Deposit w/o comp would be cruel and unusual. Unauthorized DOT transfer / seizure is pretty bad. Anything else seems impractical or too complicated.
    A possibility is to have the DOT automatically redeemed and TBTC sent to Owner, Now we are in the same state as redeemed & locked Deposits described above and can proceed. appreciate any thoughts here. @Shadowfiend

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shadowfiend re:

There are really two ways to architect this:

Redemption paths for locked deposits will be separate.

  1. redemption with locked unredeemed deposit owner token (no TBTC)
  2. redemption with redeemed deposit owner token and TBTC.

The vending machine in #364 envisions these scenarios as follows:

  1. Jus burn the NFT. (plus some additional TBTC for fees)
  2. use the wrapper: pay TBTC -> get unredeemed DOT from vending -> Deposit.requestRedemption -> burn NFT and send remaining TBTC as fees.

Note, the wrapper used in #2 just buys an NFT from Vending Machine using TBTC, then calls function used for #1

Ideally I'd like to do everything in Vending, no need for any further Deposit interactions. The vending machine handles TBTC - BTC deposit equilibrium by burning TBTC and DOTs (burning the DOTs here is good. Also leans out the vending machine

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I finally gathered + summarized what you're describing in this comment. I'm on board with this.

function draw(uint amt) {
require(amt == 1 tbtc, "partial draw not permitted");
mintTbtc(msg.sender, amt);
drawn += amt;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this make sense to track in the deposit at all?

Don't think tracking this in Deposit is the way to go. Took a while thinking about implications for having a Drawn variable or state. I'm not fully clear on whether this is intended to allow fractional drawing on larger lot sizes but assuming require(amt == 1) == require(amt == lot Size)
A benefit of the vending machine is that it also restricts minting authority to just one contract. A simple query on the NFT and Deposit state can give the vending machine all the info it needs to mint or refuse minting.

In my mind, it makes more sense for the deposit to track its own transaction's proof state

Sitting with this for a while, I think I agree. This makes sense thematically and in terms of implementation. The proof state is technically knowledge of backing UTXO's proven work state, and can justifiably be stored in deposit state. Deposit's 1-proof functionality will also be very similar to Vending Machine's 6+x-proof therefore We can cut a lot of duplicated complexity from the Vending machine.
Note: This needs to be a public / external function callable only via the vending machine.

drawn -= amt;
}

function setLocked(bool _locked) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

locked vs unlocked might be a deposit state
👍

can a deposit be locked or unlocked during courtesy call?

This is tough. quick term Deposit Owner Token = DOT
some points:
Unlocked & redeemed Deposits:

  • This is not an issue for unlocked deposits

Locked Deposits:

  • redeemed & locked Deposits: Owner controls TBTC and a redeemed DOT. If the state of the DOT (redeemed / unredeemed) is just a reflection of the contract state(as you mentioned @Shadowfiend), we can change the state of the Deposit (and therefore the DOT) during an undercollateralized state without breaching permission. The result is that the user keeps their TBTC, but loses exclusive redemption rights making their DOT worthless (since it just points to a liquidated deposit).
    TLDR - when undercolateralized, liquidation actions are open and operate as they do in the current spec. Deposit holds all state, not the NFT.
  • unredeemed Deposits: I'm not entirely sure what the best approach is here. In this situation, owner does not control any TBTC. Liquidating the linked Deposit w/o comp would be cruel and unusual. Unauthorized DOT transfer / seizure is pretty bad. Anything else seems impractical or too complicated.
    A possibility is to have the DOT automatically redeemed and TBTC sent to Owner, Now we are in the same state as redeemed & locked Deposits described above and can proceed. appreciate any thoughts here. @Shadowfiend

// check deposit qualified
totalValue = 0
unqualifiedDeposits = [...]
for deposit in unqualifiedDeposits:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes. I knew tracking volume would be impractical...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ I know this is just for demonstration and am not commenting on the code itself, just throwing ideas for how to get a number for volume for cheap. P.S i expect the same curtesy in #364 😄

maybe we could get Volume as a number updated every time a new deposit is qualified on the vending machine. We would also need a way to only count deposits created within each 6-block period. Note: this doesn't zero out, it just moved the range one block to the right. Need to figure out exactly what might affect this first. More info / digging needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this doesn't really work. Having this number on vending machine does not take into account new deposits being created, so there is a pretty large lag. Instead if we take this approach we should use the Factory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another note: doing this in factory and tracking new Deposit instances opens the door for a DoS with current funder bond levels. Now the benefit of opening new deposits with no intentions of funding has the effect of increasing block qualification requirements for a pretty long time. If the block requirement is large enough there is a possibility that the function needed for proof will be too expensive to run. Timeout logic would need revision

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the benefit of opening new deposits with no intentions of funding has the effect of increasing block qualification requirements for a pretty long time

Ah, this is what James was getting at re: minimum 1-conf tx's:

It is costless to make a 0-conf transaction that can never confirm. We're not looking for probability here, we want some evidence that the tx can confirm before we take any further on-chain action.

So one-conf makes sense here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could get Volume as a number updated every time a new deposit is qualified on the vending machine.

Take a look at my 2nd pass, attention to unqualifiedVolume 6a3fc7b

We would also need a way to only count deposits created within each 6-block period

Took a stab at implementing this. It's not perfect.

I'm wondering actually, why are we only guarding against reorgs for deposit funding, and not redemption too? cc @Shadowfiend

It seems a re-org attack could be equally as damaging on redemption as it is on minting? Consider:

  • 1000 TBTC worth of deposits are set for redemption
  • redemption tx's are sent
  • minimum difficulty accumulated, proof produced, signer bonds are released
  • reorg
  • signers sign tx with high fee (to replace-by-fee any rebroadcasting of previous redemption tx)
  • signers keep BTC and bonds

vs the funding scenario:

  • 1000 TBTC of deposits are opened
  • funding tx's are sent
  • 1000 TBTC minted
  • reorg
  • TBTC sold

It seems in the latter, the miners can directly profit, but in the former it's only the signers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this week, let’s make sure to consider this volume mechanic to be on the back burner. We’ll pick it up later. Feel free to keep ideating, of course, it’s just not the main thing we want to deliver.

Copy link
Contributor Author

@liamzebedee liamzebedee Nov 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's fair enough. Matt mentioned at planning it would be good to do it for funding as well, but not the first priority just yet.

 - remove the lock and drawing functionality
 - refer to the deposit owner token (DOT), as the token with exclusive redemption rights
 - first pass at a clean API for the vending machine
}

// After 1 conf, the DOT token can be dispensed
function dispenseDot(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vending machine shouldn't be concerned with minting owner tokens, IMO. It should solely be charged with owner token <-> TBTC exchange.

require(
_observedDiff >= _reqDiff.mul(minimumDifficultyFactor),
"Insufficient accumulated difficulty in header chain"
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flow (wherever it resides) should really be “give me a proof, I pass that proof to the deposit, the deposit updates its state, I check whether the deposit's proof level is enough for me, I mint the DOT”. Deposit state should be managed entirely within the deposit, and an “active” state should be defined purely from the deposit's perspective.

In particular, consider the vending machine to be one possible consumer of deposit owner tokens. Another consumer might require more or less proof, for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shadowfiend, Does this suggest that each Deposit has a proof state that updates dynamically for each additional proof it receives?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an update, Looks like dynamic proof states are not very desirable. Being qualified is a deposit state (ACTIVE) Reaching that state not via an incremental process, but with a single proof.

@liamzebedee
Copy link
Contributor Author

I'm going to close this since the changes are no longer current - idk if it impacts the ability to continue discussions, will reopen if necessary.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants